Feat(agent): add agent core for agent metadata #899
Conversation
Signed-off-by: Casper Nielsen <casper@diagrid.io>
Signed-off-by: Casper Nielsen <casper@diagrid.io>
CasperGN
changed the title
Signed-off-by: Casper Nielsen <casper@diagrid.io>
Signed-off-by: Casper Nielsen <casper@diagrid.io>
…nt object instead of passing Signed-off-by: Casper Nielsen <casper@diagrid.io>
Signed-off-by: Casper Nielsen <casper@diagrid.io>
Signed-off-by: Casper Nielsen <casper@diagrid.io>
Signed-off-by: Casper Nielsen <casper@diagrid.io>
Signed-off-by: Casper Nielsen <casper@diagrid.io>
Signed-off-by: Casper Nielsen <casper@diagrid.io>
Signed-off-by: Casper Nielsen <casper@diagrid.io>
Signed-off-by: Casper Nielsen <casper@diagrid.io>
Signed-off-by: Casper Nielsen <casper@diagrid.io>
…#438 Signed-off-by: Casper Nielsen <casper@diagrid.io>
Signed-off-by: Casper Nielsen <casper@diagrid.io>
Signed-off-by: Casper Nielsen <casper@diagrid.io>
Signed-off-by: Casper Nielsen <casper@diagrid.io>
…t to false Signed-off-by: Casper Nielsen <casper@diagrid.io>
Signed-off-by: Casper Nielsen <casper@diagrid.io>
Signed-off-by: Casper Nielsen <casper@diagrid.io>
Signed-off-by: Casper Nielsen <casper@diagrid.io>
…e + update modular Signed-off-by: Casper Nielsen <casper@diagrid.io>
Signed-off-by: Casper Nielsen <casper@diagrid.io>
Signed-off-by: Casper Nielsen <casper@diagrid.io>
Signed-off-by: Casper Nielsen <casper@diagrid.io>
sicoyle
left a comment
sicoyle
left a comment
There was a problem hiding this comment.
few comments for starters. I've only reviewed 10/50 files 😅
| def run_agent_conversation(): | ||
| """Run a Strands Agent with Dapr session persistence.""" | ||
|
|
||
| openai_api_key = os.getenv('OPENAI_API_KEY') |
There was a problem hiding this comment.
How can we better users trying out different LLM Providers here? Maybe warn instead of returning? What all would be needed to update if they did swap providers here?
Can you pls rm the return and just warn for this. Also, no emojis in print outs please.
| session_id = 'assistant-session-1' | ||
| agent_id = 'weather-assistant' | ||
|
|
||
| with DaprClient() as dapr_client: | ||
| session_manager = DaprSessionManager( | ||
| session_id=session_id, state_store_name='statestore', dapr_client=dapr_client | ||
| ) |
There was a problem hiding this comment.
if we removed the concept of a session in the dapr agents state revamp then should the session id from a dapr for agents POV be hidden too and this would just be the workflow id under the hood?
| """ | ||
| return self._state_store_name | ||
|
|
||
| def _register_with_agent_registry(self) -> Optional[Any]: |
There was a problem hiding this comment.
can we use a more explicit return type here?
| # agent_core extension not installed, skip registration | ||
| pass |
| if self._registry is None: | ||
| components: Sequence[RegisteredComponents] = resp.registered_components | ||
| for component in components: | ||
| if 'state' in component.type and component.name == 'agent-registry': |
There was a problem hiding this comment.
consts somewhere for the magic strings pls
| def _model_dump(self, model: BaseModel) -> Dict[str, Any]: | ||
| """Dump a Pydantic model to dict (v2/v1 support).""" | ||
| if hasattr(model, 'model_dump'): | ||
| return model.model_dump() | ||
| if hasattr(model, 'dict'): | ||
| return model.dict() | ||
| raise StateStoreError(f'Unsupported pydantic model type: {type(model)}') | ||
|
|
||
| def _ensure_dict(self, value: Any) -> Dict[str, Any]: |
There was a problem hiding this comment.
did you check if in the state pkg / classes in the python sdk if there is similar logic we can reuse instead of adding new funcs for this by chance?
|
|
||
| return self._validate_model(state_data, return_model=return_model) | ||
|
|
||
| def load_with_etag( |
There was a problem hiding this comment.
isn't this logic also a duplicate with another one of the state classes in this PR somewhere..? Can we reuse this to not have two implementations?
| metadata.setdefault('ttlInSeconds', str(ttl_in_seconds)) | ||
|
|
||
| logger.debug( | ||
| 'Saving state to %s key=%s etag=%s ttl=%s', |
| if self.mirror_writes: | ||
| self._save_local_copy(key=qualified, data=payload_dict) |
There was a problem hiding this comment.
what is the benefit of this? If I delete then it doesn't get updated, so this gets stale... If we want some sort of easy verification then why not a debug log line on state ops instead?
| def save_state_many( | ||
| service: StateStoreService, | ||
| items: Dict[str, Any], | ||
| *, | ||
| state_metadata: Optional[Dict[str, str]] = None, | ||
| state_options: Optional[Dict[str, Any]] = None, | ||
| ) -> None: | ||
| """ | ||
| Convenience wrapper for `service.save_many(...)`. | ||
| """ | ||
| service.save_many( | ||
| items, | ||
| state_metadata=state_metadata, | ||
| state_options=state_options, | ||
| ) |
There was a problem hiding this comment.
why not just make the service call itself instead of a wrapper on a single func call?
2 participants
Description
Add:
dapr-ext-agent_corepackage with shared functionality across dapr for agentsThis currently fails because I've updated references and dependencies between the ext-strands, ext-langgraph and ext-agent_core package and there is no agent_core package out yet.
@sicoyle need to be aware of the removal of
session_idfrom memory metadata.Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: N/A
External ref: dapr/dapr-agents#373
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: